Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[MCOMPILER-542] rework log and conditions to run #1

Merged
merged 1 commit into from
Nov 28, 2023

Conversation

hboutemy
Copy link

No description provided.

@hboutemy hboutemy force-pushed the jorsol-MCOMPILER-542 branch from bd9efa9 to efa180c Compare November 27, 2023 07:22
Copy link
Owner

@jorsol jorsol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review of the PR that targets a PR based on another PR.

We-have-to-go-deeper

@hboutemy hboutemy force-pushed the jorsol-MCOMPILER-542 branch from efa180c to 29967bf Compare November 28, 2023 06:57
@hboutemy hboutemy force-pushed the jorsol-MCOMPILER-542 branch from 29967bf to d14122a Compare November 28, 2023 07:13
@@ -45,10 +53,11 @@ public ModuleVisitor visitModule(String name, int access, String version) {
@Override
public void visitRequire(String module, int access, String version) {
// Check if the module name matches the java/jdk modules
if (module.startsWith("java.") || module.startsWith("jdk.")) {
if (version != null && (module.startsWith("java.") || module.startsWith("jdk."))) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This null check will preserve the current behavior when compiling from a newer JDK to an older target (eg. using JDK 11 to compile the module-info using --release 9).
In other words, it will not patch anything, while this is perfectly valid, the JDK-8318913 aims to modify this behavior to set the simple version always, I'm ok to preserve the current behavior and check later if we need to drop or not this null check.

@jorsol jorsol merged commit 8d39597 into jorsol:MCOMPILER-542 Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants